Skip to content

feat: show a shared error state when a page fails to load#1092

Merged
igboyes merged 2 commits into
mainfrom
igboyes/vir-2480-add-shared-errorstate-component-and-root-error-boundary
Jul 3, 2026
Merged

feat: show a shared error state when a page fails to load#1092
igboyes merged 2 commits into
mainfrom
igboyes/vir-2480-add-shared-errorstate-component-and-root-error-boundary

Conversation

@igboyes

@igboyes igboyes commented Jul 2, 2026

Copy link
Copy Markdown
Member

Summary

  • Add a reusable @base/ErrorState component (icon, message, optional action) that accepts the standard color palette, and render it from RouteError's retryable branch.
  • Wire an errorComponent on the root route so render failures in the root shell are caught, not just child-route failures covered by the router's defaultErrorComponent.
  • Document the shared primitive and root boundary in docs/queries.md; add tests for ErrorState.

Closes VIR-2480.

Add a reusable ErrorState component (icon, message, optional action)
and render it from RouteError's retryable branch. Wire an errorComponent
on the root route so failures in the root shell are caught too.
@linear-code

linear-code Bot commented Jul 2, 2026

Copy link
Copy Markdown

VIR-2480

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've left some high level feedback:

  • For ErrorState, consider allowing a custom icon (or making the icon optional) so that the component can be reused in contexts where a different visual treatment is needed while still sharing the layout and messaging.
  • The error icon in ErrorState currently lacks any accessibility semantics; either mark it aria-hidden if decorative or expose a meaningful aria-label/title and use a more semantic wrapper (e.g. a heading) for the main error message.
  • The hard-coded h-96 height in ErrorState might be too rigid for all layouts; consider making the height configurable or relying on padding/margins so the component adapts better to varying container sizes.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- For `ErrorState`, consider allowing a custom icon (or making the icon optional) so that the component can be reused in contexts where a different visual treatment is needed while still sharing the layout and messaging.
- The error icon in `ErrorState` currently lacks any accessibility semantics; either mark it `aria-hidden` if decorative or expose a meaningful `aria-label`/`title` and use a more semantic wrapper (e.g. a heading) for the main error message.
- The hard-coded `h-96` height in `ErrorState` might be too rigid for all layouts; consider making the height configurable or relying on padding/margins so the component adapts better to varying container sizes.

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Mark the default error icon aria-hidden since the message conveys the
meaning, and add an optional icon prop so other views can supply a
different visual.
@igboyes

igboyes commented Jul 2, 2026

Copy link
Copy Markdown
Member Author

Addressed in 9463b78:

  • Icon accessibility: default CircleAlert is now aria-hidden="true" — the <strong> message carries the meaning.
  • Icon configurability: added an optional icon prop to override the default.

Skipped:

  • Semantic heading: keeping <strong> for consistency with the sibling NotFound component; a route-level error isn't necessarily the page heading.
  • h-96: already overridable — className is merged last via cn, so callers can replace the height.

@igboyes igboyes merged commit 3fd1fe8 into main Jul 3, 2026
11 checks passed
@igboyes igboyes deleted the igboyes/vir-2480-add-shared-errorstate-component-and-root-error-boundary branch July 3, 2026 00:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant